Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pycurl: Add curl as a depedency #1284

Merged
merged 4 commits into from
Oct 26, 2023
Merged

Conversation

bzadm
Copy link
Contributor

@bzadm bzadm commented Aug 23, 2023

Adds support for pycurl via override. Tested on GNU/Linux with NixOS. Fixes #1283

Test

pyproject.toml:

[tool.poetry]
name = "pycurl-test"
version = "0.1.0"
description = ""
authors = ["nhr-zib <[email protected]>"]

[tool.poetry.dependencies]
python = "^3.10"
pycurl = "7.45.2"

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"

flake.nix:

{
  description = "pycurl test";

  inputs = {
    flake-utils.url = "github:numtide/flake-utils";
    nixpkgs.url = "github:NixOS/nixpkgs/nixos-23.05";
    poetry2nix = {
      url = "github:bzadm/poetry2nix/master";
      inputs.nixpkgs.follows = "nixpkgs";
      inputs.flake-utils.follows = "flake-utils";
    };
  };

  outputs = { self, poetry2nix, nixpkgs, flake-utils }:
    flake-utils.lib.eachDefaultSystem (system:
      let pkgs = nixpkgs.legacyPackages.${system};
      in {
        devShells.default = pkgs.mkShell {
          buildInputs = [
            (poetry2nix.legacyPackages.${system}.mkPoetryEnv {
              pyproject = ./pyproject.toml;
              poetrylock = ./poetry.lock;
            })
          ];
        };
      });
}

test.py:

# See http://pycurl.io/docs/latest/quickstart.html#retrieving-a-network-resource
import pycurl
import re
try:
    from io import BytesIO
except ImportError:
    from StringIO import StringIO as BytesIO

headers = {}
def header_function(header_line):
    # HTTP standard specifies that headers are encoded in iso-8859-1.
    # On Python 2, decoding step can be skipped.
    # On Python 3, decoding step is required.
    header_line = header_line.decode('iso-8859-1')

    # Header lines include the first status line (HTTP/1.x ...).
    # We are going to ignore all lines that don't have a colon in them.
    # This will botch headers that are split on multiple lines...
    if ':' not in header_line:
        return

    # Break the header line into header name and value.
    name, value = header_line.split(':', 1)

    # Remove whitespace that may be present.
    # Header lines include the trailing newline, and there may be whitespace
    # around the colon.
    name = name.strip()
    value = value.strip()

    # Header names are case insensitive.
    # Lowercase name here.
    name = name.lower()

    # Now we can actually record the header name and value.
    # Note: this only works when headers are not duplicated, see below.
    headers[name] = value

buffer = BytesIO()
c = pycurl.Curl()
c.setopt(c.URL, 'http://pycurl.io')
c.setopt(c.WRITEFUNCTION, buffer.write)
# Set our header function.
c.setopt(c.HEADERFUNCTION, header_function)
c.perform()
c.close()

# Figure out what encoding was sent with the response, if any.
# Check against lowercased header name.
encoding = None
if 'content-type' in headers:
    content_type = headers['content-type'].lower()
    match = re.search('charset=(\S+)', content_type)
    if match:
        encoding = match.group(1)
        print('Decoding using %s' % encoding)
if encoding is None:
    # Default encoding for HTML is iso-8859-1.
    # Other content types may have different default encoding,
    # or in case of binary data, may have no encoding at all.
    encoding = 'iso-8859-1'
    print('Assuming encoding is %s' % encoding)

body = buffer.getvalue()
# Decode using the encoding we figured out.
print(body.decode(encoding))

Test the build and functionality:

$ nix develop -i -c python test.py
[ ... expected HTML output ]

@@ -1853,6 +1853,13 @@ lib.composeManyExtensions [
}
);

pycurl = super.pycurl.overridePythonAttrs (
old: {
propagatedBuildInputs = (old.propagatedBuildInputs or [ ]) ++ [ pkgs.curl ];
Copy link
Contributor

@andersk andersk Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not need to be propagated—after PycURL is built, the C headers for curl are not needed to use PycURL.

pycurl = super.pycurl.overridePythonAttrs (
old: {
propagatedBuildInputs = (old.propagatedBuildInputs or [ ]) ++ [ pkgs.curl ];
nativeBuildInputs = (old.nativeBuildInputs or [ ]) ++ [ pkgs.curl ];
Copy link
Contributor

@takeda takeda Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to the above comment. I believe all you need is just specify buildInputs and add pkgs.curl there.

nativeBuildInputs is meant for tooling that are typically tools that are needed during build time. Its purpose is clearer when you do cross compiling. So you would specify there things like cmake, for python it would be setuptools, poetry-core etc.

BTW: I see netcdf4 using propagatedBuildInputs but that likely is incorrect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may need to be native for the curl-config script. (But still needn’t be propagated.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the solution is to add curl as both a build input and a native input.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, my bad.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review and explanation regarding the build inputs. I'll try to make the changes tomorrow.

@@ -1853,6 +1853,13 @@ lib.composeManyExtensions [
}
);

pycurl = super.pycurl.overridePythonAttrs (
old: {
buildInputs = (old.propagatedBuildInputs or [ ]) ++ [ pkgs.curl ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bzadm you should also rename old.propagatedBuildInputs to old.buildInputs

Copy link
Contributor

@l0b0 l0b0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Would you also be willing to create tests for this package? If so, see

  • tests/default.nix for the test entry and
  • the tests/mutmut directory for a module test or tests/matplotlib-post-3-7 for a library test.

@adisbladis adisbladis enabled auto-merge October 26, 2023 03:05
@adisbladis adisbladis merged commit 49ba4ae into nix-community:master Oct 26, 2023
118 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pycurl: add curl as a dependency
6 participants